server: fix artifact handling reset on restart #444
Conversation
luatest/server.lua
Outdated
|
|
||
| local prefix = fio.pathjoin(Server.vardir, 'artifacts', self.rs_id or '') | ||
| self.artifacts = fio.pathjoin(prefix, self.id) | ||
| if fio.path.exists(self.artifacts) then |
There was a problem hiding this comment.
Why do we have to delete the artifacts on restart? Artifacts should be saved only when the server is dropped, no?
Also, the comment to server:stop() seems outdated - it says that artifacts are saved only on test failures but it looks like they are saved unconditionally.
There was a problem hiding this comment.
Artifacts should be saved only when the server is dropped, no?
Yes, I think you're right. Fixed.
mandesero
force-pushed
the
mandesero/gh-409-server-artifacts
branch
from
ac276a5 to
a299af1
Compare
mandesero
force-pushed
the
mandesero/gh-409-server-artifacts
branch
from
a299af1 to
18ceda0
Compare
mandesero
force-pushed
the
mandesero/gh-409-server-artifacts
branch
2 times, most recently
from
48f2121 to
342acf7
Compare
Fix a bug when server initialization didn't reset artifact handling, which caused stale artifacts to persist after server restarts. Closes tarantool#409
mandesero
force-pushed
the
mandesero/gh-409-server-artifacts
branch
from
342acf7 to
29e8538
Compare
|
|
||
| if (node.had_failure or not node:is('success')) and utils.table_len(node.servers) > 0 then | ||
| for _, server in pairs(node.servers) do | ||
| server:save_artifacts() |
There was a problem hiding this comment.
AFAIU this is invoked after running after_test and after_each hooks. Is it possible to save artifacts before running the hooks? The point is that the hooks may do some cleanup, for example, drop test spaces, etc, while we'd usually like to see what was the actual server state at the moment of failure.
| assert_artifacts_path(g.s_all) | ||
| assert_artifacts_path(g.s_all2) | ||
| ctx.runner:update_status(test, {status = 'fail'}) | ||
| test:update_status('success') |
There was a problem hiding this comment.
These tests look weird. Why do we need to change the status manually? Can't we fail an assertion or raise an error to check if artifacts are saved on failure?
2 participants
Fix a bug when server initialization didn't reset artifact handling, which caused stale artifacts to persist after server restarts.
Closes #409